-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[platform/cel]: Add Celestica Silverstone device to 201811 branch #5424
base: 201811
Are you sure you want to change the base?
Conversation
…sonic-net#3107) * sonic-device-data: update SAI config checker for Broadcom TD3 and TH3 The following properties have been approved by the Broadcom chip arch team: l3_alpm_ipv6_128b_bkt_rsvd ifp_inports_support_enable pll_bypass dpr_clock_frequency device_clock_frequency port_flex_enable mmu_port_num_mc_queue serdes_core_rx_polarity_flip_physical{<PORT>} serdes_core_tx_polarity_flip_physical{<PORT>} Signed-off-by: Dante (Kuo-Jung) Su <dante.su@broadcom.com> Change-Id: I1c6239cddfb0582a9298e671d792a32f79e4f006
This pull request introduces 1 alert when merging f017c78 into 7e6fa15 - view on LGTM.com new alerts:
|
…-net#3591) * [platform/broadcom] Celeatica Silverstone add IPMI platform sensor read. * [platform_sensors] Silverstone update temperature sensor description
This pull request introduces 1 alert when merging 5dcdaa2 into 7e6fa15 - view on LGTM.com new alerts:
|
This pull request introduces 7 alerts when merging 7faf801 into 4c80d99 - view on LGTM.com new alerts:
|
@@ -0,0 +1,129 @@ | |||
# name lanes alias index speed | |||
Ethernet0 33,34 QSFP1/1 1 100000 | |||
Ethernet1 35,36 QSFP1/2 1 100000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Ethernet2 instead of Ethernet1. We should reserve one interface per lane.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you, I will update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in 2d79cb5
@@ -0,0 +1,33 @@ | |||
# name lanes alias index speed | |||
Ethernet0 33,34,35,36,37,38,39,40 QSFPDD1 1 400000 | |||
Ethernet4 41,42,43,44,45,46,47,48 QSFPDD2 2 400000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Ethernet8 instead of Ethernet4. We should reserve one interface per lane.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you, I will update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in 2d79cb5
@@ -0,0 +1,9 @@ | |||
#The Port LED of Silverstone SONIC can't work well, after the issue is fixed by SAI, The led will start. | |||
#led auto on; led start | |||
rcload /usr/share/sonic/hwsku/pre-emphasis_PAM4_optics.soc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be separate out from led process. Please add this into start.sh, following how it does for led.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean this start.sh , is that right ?
https://github.com/Azure/sonic-buildimage/blob/201811/platform/broadcom/docker-syncd-brcm/start.sh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 6ac2ff7
Please review it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be removed from here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed in a01897c
silverstone/scripts/platform_sensors.py usr/local/bin | ||
silverstone/cfg/silverstone-modules.conf etc/modules-load.d | ||
silverstone/systemd/platform-modules-silverstone.service lib/systemd/system | ||
silverstone/modules/sonic_platform-1.0-py2-none-any.whl usr/share/sonic/device/x86_64-cel_silverstone-r0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this is required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to support reboot cause service
https://github.com/Azure/sonic-buildimage/blob/201811/files/image_config/process-reboot-cause/process-reboot-cause
ref : #3198
@@ -24,6 +25,7 @@ ext_tcam_freq | |||
force_core_pll | |||
fpem_mem_entries | |||
higig2_hdr_mode | |||
ifp_inports_support_enable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change is needed? Same thing for below checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in 2d79cb5
[device/celestica]: update silverstone device config
[syncd-brcm]: add pre-emphasis load to start.sh
@@ -0,0 +1,9 @@ | |||
#The Port LED of Silverstone SONIC can't work well, after the issue is fixed by SAI, The led will start. | |||
#led auto on; led start | |||
rcload /usr/share/sonic/hwsku/pre-emphasis_PAM4_optics.soc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be removed from here.
@@ -38,6 +38,12 @@ fi | |||
|
|||
supervisorctl start syncd | |||
|
|||
# If this platform has a pre_emphasis setting file, load it | |||
if [[ -r $HWSKU_DIR/pre-emphasis_PAM4_optics.soc ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this was warm-rebooted, should we skip this one? does it impact traffic by loading this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in befd2ca
@@ -38,6 +38,12 @@ fi | |||
|
|||
supervisorctl start syncd | |||
|
|||
# If this platform has a pre_emphasis setting file, load it | |||
if [[ -r $HWSKU_DIR/pre-emphasis_PAM4_optics.soc ]]; then | |||
wait_syncd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait_syncd should be done ONLY once at maximun.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in befd2ca
Please add the test log with the image build from 201811 and this PR (Not images with other changes). |
@zhenggen-xu , The test log uploaded at https://github.com/Azure/sonic-buildimage/files/5414680/sonic-broadcom-201811_silverstone.log |
@@ -0,0 +1,8 @@ | |||
#The Port LED of Silverstone SONIC can't work well, after the issue is fixed by SAI, The led will start. | |||
#led auto on; led start | |||
sleep 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sleep is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed in d6b6a8d
retest broadcom please |
retest vsimage please |
@yxieca , Please review and merge this PR |
- What I did
- How I did it
- How to verify it